CHANGELOG.unreleased.md (1)

22-22: LGTM, but consider a minor language improvement.

Clearly explains the new behavior of zeroing out data outside the defined bounding box. The explanation is precise and matches the PR's objectives.

Consider simplifying "outside of the bounding box" to just "outside the bounding box" to avoid redundancy:

- When loading data from a data layer that has data stored beyond the bounding box specified in the datasource-properties.json, data outside of the bounding box is now zeroed. (the layer is "clipped"). [#8551](https://github.com/scalableminds/webknossos/pull/8551)
+ When loading data from a data layer that has data stored beyond the bounding box specified in the datasource-properties.json, data outside the bounding box is now zeroed. (the layer is "clipped"). [#8551](https://github.com/scalableminds/webknossos/pull/8551)

🧰 Tools

🪛 LanguageTool

[style] ~22-~22: This phrase is redundant. Consider using “outside”.
Context: ...in the datasource-properties.json, data outside of the bounding box is now zeroed. (the la...

(OUTSIDE_OF)

util/src/main/scala/com/scalableminds/util/geometry/BoundingBox.scala (2)

48-50: Avoid temporary allocation in isFullyContainedIn for a micro-performance win
intersection allocates a new BoundingBox just to compare against this.
For a hot path you can sidestep the allocation by comparing coordinates directly:

- def isFullyContainedIn(other: BoundingBox): Boolean =
-   this.intersection(other).contains(this)
+ def isFullyContainedIn(other: BoundingBox): Boolean =
+   topLeft.x >= other.topLeft.x &&
+   topLeft.y >= other.topLeft.y &&
+   topLeft.z >= other.topLeft.z &&
+   bottomRight.x <= other.bottomRight.x &&
+   bottomRight.y <= other.bottomRight.y &&
+   bottomRight.z <= other.bottomRight.z

This keeps the method allocation–free while preserving semantics.

---

67-69: Minor naming nitpick – consider translate instead of move
The new helper is great, but move now exists in both BoundingBox and Vec3Int with slightly different semantics (in-place vs. returning a new box). Using a verb like translate or shift would reduce cognitive load and avoid confusing it with the Vec3Int counterpart.

webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/BinaryDataService.scala (1)

111-139: clipToLayerBoundingBox copies row-wise but still triple-loops ‑ can be flattened
Great to see explicit clipping! The inner copy already works per X-row; you can also flatten Y/Z loops by computing linear slice offsets and invoking a single System.arraycopy per contiguous block.
This typically halves the loop overhead for large requests:

- for {
-   z <- intersection.topLeft.z until intersection.bottomRight.z
-   y <- intersection.topLeft.y until intersection.bottomRight.y
- } {
-   val offset = (intersection.topLeft.x +
-                 y * requestBboxInMag.width +
-                 z * requestBboxInMag.width * requestBboxInMag.height) * bytesPerElement
-   System.arraycopy(inputArray,
-                    offset,
-                    outputArray,
-                    offset,
-                    (intersection.bottomRight.x - intersection.topLeft.x) * bytesPerElement)
- }
+val rowBytes = (intersection.bottomRight.x - intersection.topLeft.x) * bytesPerElement
+val sliceBytes = requestBboxInMag.width * requestBboxInMag.height * bytesPerElement
+val yStride   = requestBboxInMag.width * bytesPerElement
+
+var z = intersection.topLeft.z
+while (z < intersection.bottomRight.z) {
+  var y = intersection.topLeft.y
+  val zOffset = z * sliceBytes
+  while (y < intersection.bottomRight.y) {
+    val offset = zOffset + y * yStride + intersection.topLeft.x * bytesPerElement
+    System.arraycopy(inputArray, offset, outputArray, offset, rowBytes)
+    y += 1
+  }
+  z += 1
+}

No change in behaviour, ~2-3× speed-up on large volumes.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

CHANGELOG.unreleased.md (2)

21-21: Use consistent quoting for UI elements
Consider using backticks around the Create Animation job name to follow the existing convention (e.g., backticks around View Modes in other entries).

Apply this diff:

- - Replaced fixed threshold of 40 meshes by a dynamic limit based on the number of triangles in the mesh for the "Create Animation" job. [#8588]
+ - Replaced fixed threshold of 40 meshes by a dynamic limit based on the number of triangles in the mesh for the `Create Animation` job. [#8588]

---

25-25: Refine phrasing and remove redundancy
The phrase “outside of the bounding box” is redundant per our style guide and the parentheses break sentence flow. Also replace curly quotes with straight quotes.

Apply this diff:

- - When loading data from a data layer that has data stored beyond the bounding box specified in the datasource-properties.json, data outside of the bounding box is now zeroed. (the layer is “clipped”). [#8551]
+ - When loading data from a data layer that has data stored beyond the bounding box specified in the datasource-properties.json, data outside the bounding box is now zeroed, clipping the layer. [#8551]

🧰 Tools

🪛 LanguageTool

[style] ~25-~25: This phrase is redundant. Consider using “outside”.
Context: ...in the datasource-properties.json, data outside of the bounding box is now zeroed. (the la...

(OUTSIDE_OF)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro